Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix deleting unused images #427

Merged
merged 5 commits into from
Sep 30, 2023
Merged

Conversation

julesvirallinen
Copy link
Contributor

closes #391

There was a bug in publisher where it URI encoded the image path, causing images not to be deleted.

The new publication center also did not include images.

Both issues are fixed.

@julesvirallinen julesvirallinen changed the title testVault: add unused image Fix deleting unused images Sep 26, 2023
@oleeskild
Copy link
Owner

Thank you 🙌 Can you resolve the merge conflicts that is now a thing after accepting the other PR?
Also, I think this causes an issue. If two published notes are embedding the same image, we don't want to delete the image. This means we can't really delete an image before checking every single other published note.

A potential fix for this could be to always append the note path behind the image when uploading to github. We would then risk having duplicate images. But I think trading more storage use for faster publishing is a better solution. Storage is cheap (or in this case, free).

What do you think?

@julesvirallinen
Copy link
Contributor Author

@oleeskild of course, I prefer to revase onto target branch as it feels cleanest, don't mind doing it ever

Not sure that's an issue, ill see if I can recreate in test garden

@oleeskild oleeskild merged commit 8fe967b into oleeskild:main Sep 30, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

unused image still in site/img/user folder
2 participants